Add VDO tests converted from vdo-devel perl tests#5
Conversation
|
currently this branch is based on PR #4 and incorporates those changes |
|
(sigh) Looks like the Claude-driven refactoring broke some intermediate versions, like changing full_tests.py to import get_free_blocks from stats.py in one commit, but the adding of get_free_blocks to stats.py happens later. Look for a regrouping of patches coming soon. The final contents should be the same though. |
|
The newer push is basically the same total overall content (plus one bugfix, using "--force" with blkdiscard because one test issues calls in parallel), but regrouped to a more sensible patch set (no calling a function before introducing it). Still based on PR #4. |
|
Also this changeset really wants Hank's tagging feature so that we can specify that some of the tests should not be run by default, only if explicitly requested. A couple stress tests are really slow. |
There was a problem hiding this comment.
Okay, I didn't read the details of any of the new tests, just the general shape of the series. Some thoughts:
a) It's worth thinking about the right level for the various infrastructure improvements; several of them would be useful for any dm target, while others are more specific to VDO or even to particular tests. (I've called out some of these cases.)
b) I think infrastructure improvements and new test should not be in the same commits. Though I could see having a small series that is (infrastructure to implement certains tests / add the tests).
c) Some of the test grouping seem like catch-alls instead of logical grouping; in my opinion that makes it harder to properly evaluate any of the test (versus splitting them into separate commits).
d) It's probably also worth splitting the series as a whole by breaking out several smaller series in order to make review tractable, and so we can commit separate tests as they get approved.
e) For the specific tests, I think it's asking whether all of these tests are still valuable or whether we should cut down on the volume. Do we really need 3-4 variations on GenData tests? etc.
f) For test that are valuable, I think it's worth having more detail in the commit messages in order to help a reviewer understand which original perl tests are being migrated, what was changed, dropped, or consolidate compared to the original test, and why that's appropriate. As a reviewer, if I see a change in behavior between the original perl test and the new one, I would like to understand if it was intentional or not, and what the effect of the change is on our test coverage, and our test resource cost.
| raise AssertionError("VDO not online within 30 seconds") | ||
|
|
||
|
|
||
| def wait_until_packer_only(vdo): |
There was a problem hiding this comment.
Can we find a better name for this function? Especially if it's going to be in a generic utils location, it should really describe its goal so new test writers can know whether it's appropriate. (The expanded comment is good, can we capture some that idea in the function name?)
| if status.vdo_status(dev)["index-state"] != "online": | ||
| raise AssertionError("VDO not online within 30 seconds") | ||
|
|
||
|
|
There was a problem hiding this comment.
Also a bit of hobbyhorse for me, but can you check for double blank lines? You've added them both before and after this new function.
There was a problem hiding this comment.
Yes, that's PEP 8, the Python coding style standard. Most but not all of dmtest outside of 'vdo' conforms. We should be following it.
| return tuple(int(p) for p in parts) | ||
|
|
||
|
|
||
| def _get_vdo_version(): |
There was a problem hiding this comment.
This version manipulation feels a little awkward (does returning the version as a tuple provide any real value, over,say, just an int?), but if this kind of version parsing is useful, it's probably better off being elevated to block_dev of something since it seems equally useful for any dm target.
| return None | ||
|
|
||
|
|
||
| def vdo_min_version(min_version_str): |
There was a problem hiding this comment.
Version comparison, similarly, seems potentially useful for any dm target.
| point removal on exit. | ||
| """ | ||
| if fs_class is None: | ||
| fs_class = Ext4 |
There was a problem hiding this comment.
Why use Ext4 as the default filesystem, when the Fedora default (iirc) is XFS?
This function also seesm like a utility that any dm target test could theoretically benefit from.
| @@ -3,8 +3,11 @@ | |||
| import dmtest.vdo.vdo_stack as vs | |||
There was a problem hiding this comment.
The commit message makes this commit feel overstuffed, maybe break this into a couple smaller, clearer functional improvements? It also mentions wait_until_packer_only(), which is not here. (It's in a prior commit.)
| byte_offset = self.block_size * self.offset | ||
| byte_size = self.block_size * self.block_count | ||
| process.run(f"blkdiscard -o {byte_offset} -l {byte_size} {self.path}") | ||
| process.run(f"blkdiscard --force -o {byte_offset} -l {byte_size} {self.path}") |
There was a problem hiding this comment.
It's weird having tool improvements interspersed with new tests. I'd prefer to see tool and utility fixes and improvements described and justified on their own merits. I think it's too easy to overlook the broader implications of a change if it's not the main focus of the commit.
There was a problem hiding this comment.
I figured this was a bugfix (to make concurrent invocations work) needed for one of the tests, not a significant improvement on its own.
| @@ -0,0 +1,334 @@ | |||
| """Tests VDO sysfs interface.""" | |||
There was a problem hiding this comment.
We barely have sysfs parameters any more, right? Is it really worth adding sysfs tests?
| import dmtest.process as process | ||
|
|
||
|
|
||
| SECTOR_SIZE = 512 |
There was a problem hiding this comment.
I was just planning to add constants like "SECTOR_SIZE" to utils.py.
There was a problem hiding this comment.
I noticed since that it's already defined elsewhere
| @@ -0,0 +1,219 @@ | |||
| """VDO memory allocation failure test. | |||
There was a problem hiding this comment.
MemoryFail is such a huge test, might that be a better candidate for sts testing?
There was a problem hiding this comment.
It only works with our internal version; I sort of assumed STS tests would be applied mainly to released versions?
Add mounted_fs() for filesystem test setup with automatic cleanup, and the SLAB_BITS_SMALL constant. Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Converts Basic01.pm, BasicFSDedupe.pm, Create03.pm, and UDSTimeout01.pm to Python. basic_fs_dedupe_tests.py (from BasicFSDedupe.pm): - Disables lazy_itable_init on ext4 (via new fs.py enhancement) to prevent background metadata writes from skewing dedup stats - Tolerance widened for statistics verification: 0.1/0.05 vs Perl's 0.01 uds_timeout_tests.py (from UDSTimeout01.pm): - Perl uses configurable 100ms VDO timeout; Python relies on upstream dm_vdo's fixed 5000ms timeout exceeded by 6000ms dm-delay read delay - Dataset size reduced: 10 x 20,000 blocks (200K total) vs Perl's 10 x 150,000 (1.5M total) - Two-phase approach: populate UDS index on fast storage first, then restart VDO on dm-delay device - Uses dm-delay table-swap technique to avoid udev probe hang - Disables delay after 60 seconds to let remaining I/O drain quickly Infrastructure: - fs.py: Added lazy_itable_init parameter for ext4 formatting Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Converts Full01.pm, Full02.pm, Full03.pm, Full04.pm, and FullWarn.pm to Python. Full02 and Full04 consolidated into one file. full_tests.py (merged Full01.pm into existing test): Rather than creating a separate file, Full01's fill-and-verify logic was merged into the pre-existing full_tests.py, expanding its simple fill test into comprehensive boundary testing: nearly-full dedupe, completely-full dedupe, ENOSPC on new data, stats verification after failed writes, and trim/discard space reclamation. Uses slab_bits=13 (vs 15) and 3GB (vs 375MB). Adds populate_block_map() before testing. full_02_tests.py (from Full02.pm + Full04.pm, consolidated): Merges direct I/O (Full02) and page cache (Full04) variants into one file with a direct parameter. Uses 3GB vs 375MB physical. Page cache error recovery differs: Python uses _sync_device_ignoring_errors() + drop_caches; Perl restarts the device. full_03_tests.py (from Full03.pm): - Uses 3GB vs 375MB physical storage full_warn_tests.py (from FullWarn.pm): - Uses 3GB vs 375MB physical storage - Simplified: only verifies the device reports full; does not test multi-level warning system (50%, 75%, 90%, 100%) Not converted: - FullWarnDmEventd.pm: Requires dmeventd integration for fullness warning monitoring Infrastructure: - gendatablocks.py: Added direct I/O support (O_DIRECT + mmap-aligned buffers); added --force to blkdiscard in trim() - stats.py: Added get_usable_data_blocks() and get_free_blocks() helpers Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Converts Direct01.pm through Direct06.pm. Direct03 split into 3 independent Python tests (8 tests total). direct_03_tests.py (from Direct03.pm): - Single Perl method split into 3 independent Python tests - Discard bio count check simplified to > 0 instead of exact count based on ceil(blockCount / maxDiscardBlocks) direct_06_tests.py (from Direct06.pm): - Omits final verification step where Perl verifies slice2 reads zeros after trim Potential bugs: - direct_05_tests.py: Uses direct=True but Perl uses buffered I/O (fsync => 1); memory.md confirms Direct05 should NOT use O_DIRECT. Also uses echo 1 for drop_caches vs echo 3 elsewhere. - direct_06_tests.py: Step 10 comment says "using fsync instead of direct" but code uses direct=True Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Move the function from compress_tests.py to utils.py so it can be shared by other test modules, and rename it to wait_until_io_settled(). Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Converts Collide01-03.pm, Compress01.pm, and InFlightDedupeAndCompress.pm. Three Collide files consolidated into one. collide_tests.py (from Collide01.pm, Collide02.pm, Collide03.pm, consolidated): No functional differences beyond consolidation. compress_tests.py (from Compress01.pm): - Block count: 1024 (4MB) vs Perl's 5000 - Checks additional stats: posts not found, posts found, bios in write - Omits data verification after second write - Compression ratio tolerance uses (size_in_blocks + 2) // 3 to account for packer flush artifacts Not converted: - Compress02.pm: Statistical variance analysis running compression 20 times with result aggregation; custom suite builder doesn't fit dmtest framework Infrastructure: - murmur3collide.py: Pure Python MurmurHash3_x64_128 implementation with collision generation, replacing the C murmur3collide tool Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Converts GenData01.pm through GenData04.pm. gen_data_04_tests.py (from GenData04.pm): - Write/verify pattern changed: Python writes all datasets in parallel then verifies serially; Perl performs write-and-verify atomically per dataset Infrastructure: - dataset_helpers.py: Shared write_file_dataset() / verify_file_dataset() functions consolidating the Perl genDataFiles() wrapper Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Converts Discard512.pm, Discard512Compressed.pm, and Zero01.pm. Two Discard512 files consolidated into one. discard_512_tests.py (from Discard512.pm + Discard512Compressed.pm, consolidated): - Shared _run_discard_test(fix, compressed) helper replaces Perl's class inheritance - Different compressible data generation: Python writes 90% 0xFF + 10% random per 512-byte sector; Perl uses generateDataFile(compressibility => 90) zero_tests.py (from Zero01.pm): Tests omitted: - testLogical: tests non-VDO linear device (framework validation) - testBasic: tests default device type (framework validation) Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Converts CompressDedupeDefaults.pm, VDOCompressFlag.pm, VDOLoadFailure01.pm, VDOLoadFailure02.pm, MajorMinor.pm, and SlabCount01.pm into 4 Python test files. Two consolidations: CompressDedupeDefaults.pm + VDOCompressFlag.pm, and VDOLoadFailure01.pm + VDOLoadFailure02.pm. compress_dedupe_flags_tests.py (from CompressDedupeDefaults.pm + VDOCompressFlag.pm, consolidated): - Python tests runtime toggling (messages and table reloads); Perl tests default values at creation time and persistence across reboots - Does not test persistence across reboots or verify compressed-blocks-written statistics Tests omitted: - VDOCompressFlag testOn/testOff: require machine reboot - CompressDedupeDefaults testDefault*/testEnable*OnCreate/testDisable*OnCreate: creation-time default testing load_failure_tests.py (from VDOLoadFailure01.pm + VDOLoadFailure02.pm, consolidated): - t_mixed_zone_counts extracted from Perl's testIllegalValues into separate test Tests omitted: - VDOLoadFailure02 testOutOfMemory: progressively increasing block_map_cache_size; environment-dependent major_minor_tests.py (from MajorMinor.pm): - Uses linear device for backing storage slab_count_tests.py (from SlabCount01.pm): - Storage sizes and expectations differ due to 20GB test device; Python uses range checks instead of exact slab counts: - t_tiny_tiny: 3GB/1-3 slabs (Perl: 375MB/exactly 1) - t_tiny_multi: 4.3GB/8-15 slabs (Perl: 750MB/exactly 5) - t_tiny_small: 12GB/60-80 slabs (Perl: 5GB/exactly 39) - t_small_small: 4GB/>=2 (Perl: 2GB/>=2) Tests omitted: - testSmall: needs 276GB+ storage - testLarge: needs 35GB+ storage Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Converts DeviceSwap.pm, Dmsetup.pm, Dual01.pm, Instance.pm, and VDORename.pm into 5 Python test files. device_swap_tests.py (from DeviceSwap.pm): - Python uses 3GB linear devices; Perl uses 10GB LVM volumes dmsetup_tests.py (from Dmsetup.pm): - t_basic_ops skips growLogical sub-test Tests omitted: - testIndex: complex 9-step index state machine test - testMultiVdoDefiningTable: multiple VDO targets in one table - testOptionalParameters: VDO with no optional parameters - testVersions: table format versions 0-4 - testReloadInactiveTable: BZ 1669960 / VDO-4433 - testSameStorage: two VDOs on same storage (BZ 1725052 / VDO-4691) - testBackingDeviceSizeMismatch: BZ 1732922 / VDO-4718 - testEmptyTable: empty parameter set - testBackingDeviceChange: backing device swap via stripfua-linear dual_tests.py (from Dual01.pm): - Converts dual VDO device discard-during-write test instance_tests.py (from Instance.pm): - Python uses 3 thin volumes of 6.6GB; Perl uses 3 LVM logical volumes rename_tests.py (from VDORename.pm): - Tests VDO device renaming using dmsetup rename Infrastructure: - device_mapper/interface.py: Added rename() function Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Converts Sysfs.pm into sysfs_tests.py. sysfs_tests.py (from Sysfs.pm): - Version check simplified: logs version only, no assertion - Non-root permission testing omitted Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Converts GrowLogical01-03.pm. grow_logical_02_tests.py (from GrowLogical02.pm): - Simulated power cycle (unmount, cache drop, fsck, VDO restart) replaces Perl's actual machine reboot Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Converts GrowPhysical01.pm and GrowPhysical03.pm. grow_physical_01_tests.py (from GrowPhysical01.pm): Tests omitted: - testNoGrowth: tested LVM-specific behavior (LVM rejects same-size resize; VDO accepts it as no-op) Not converted: - GrowPhysical02.pm: Requires dory fault injection for crash simulation. Scenarios 2 and 4 use dirty shutdown to verify recovery journal replay; clean shutdown would not exercise the recovery path. - GrowFailure01.pm: Memory exhaustion via 50TB growth increments until OOM. Environmental dependency makes it unsuitable for a standard test suite. Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Add _parse_version(), _get_vdo_version(), and vdo_min_version() to utils.py for use as a test dependency check. Tests that require a minimum VDO kernel module version can use vdo_min_version() as a dep_fn to skip gracefully on older kernels. Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Converts VDOFormatInKernel.pm (testOptions, testMinimumSize, testDirtyStorage). Requires VDO >= 9.2.0. vdo_format_in_kernel_tests.py (from VDOFormatInKernel.pm): t_options -- reduced parameter validation: Omits negative slab bits (-2), maxUInt/beyond-maxUInt values, 4TB logical size, sizes exceeding 4PB max, and extremely large logical sizes (maxUInt * KB/MB/GB/PB/TB). Smaller test device (~20GB vs 50GB) prevents sparse index testing (sparse="on" not tested). t_minimum_size -- simplified: Validates that undersized formatting fails with correct error but does not extend device to exact minimum and retry (Perl extends to minimum-1 to verify failure, then to minimum to verify success). t_dirty_storage -- differences: Writes 1GB (vs Perl's full device fill). Explicitly zeros first block before formatting (kernel formatting requires zeroed first block to detect unformatted storage). Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Converts MemoryFail01.pm testStart only. memory_fail_01_tests.py (from MemoryFail01.pm, MemoryFailBase.pm): - Adds MAX_ALLOCATION_FAILURE_PASSES cap (default None = exhaustive); Perl supports firstPass/lastPass range selection instead - Index state checking simplified: checks "online"/"opening" without waiting or asserting full dedup functionality - Memory tracking stubbed out (no-ops); Perl conditionally enables via trackMemory property - Explicitly calculates/reports allocation count during startup; Perl doesn't Infrastructure: - process.py: Error messages now include command name and exit status Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
Adds thread_config_tests.py to verify that VDO creates the correct kernel threads for different thread/zone count configurations. t_single_thread_mode checks that the default configuration (all zone counts zero) uses a single "reqQ" thread. t_thread_counts varies each thread parameter independently (logical, physical, hash zone counts and bio, ack, cpu thread counts) while holding others at their defaults, verifying the full set of kernel thread counts via ps output after each configuration change. Assisted-by: Claude Opus 4.6 <claude-opus-4-6>
2 participants
No description provided.